Repository navigation
perf(sdk): stop repeating bundle hashing and teardown state reads - #12974
Merged
Merged
Conversation
Every CLI process verifies the bundle before its first operation. The linux_arm64 bundle lists 365 MB in 12 files, and hashing them one at a time took 190-360 ms per process. Hash them on several threads instead; the first failure in manifest order still decides the error.
Destroy and plan-destroy read every stage's bindings with tofu init and show, then read them again before planning the stage: two more OpenTofu processes per stage, about 150 ms each. Nothing changes that state in between, so plan each stage from the bindings already read.
Contributor
|
v1 documentation preview: https://nvidia-preview-nemoclaw-v1-pr-12974.docs.buildwithfern.com/nemoclaw |
cv
added a commit
that referenced
this pull request
Oct 11, 2026
Part of #12882. ## Failure The pinned `kreuzwerker/docker` provider waits 2 s before every `docker_network` read and after every removal. Every runtime plan, apply, export, or destroy reads the network once it exists, and a destroy also waits for the removal. Only the `remote_service` lifecycle tests deploy a service with a Docker network: the SSH simulator is the only fake engine that serves `/networks`. I counted the network reads per test from a request log in the SSH simulator (local `linux_arm64` bundle; the logging is not part of this change). The 12 tests made 75 reads, about 150 s of their 404 s of test time: | Test | Reads | |---|---| | `managed_bearer_storage_or_key_damage_stops_plan_and_apply_without_changing_bindings` | 13 | | `remote_model_reapply_and_drift_keep_bindings_and_stop_on_observation_failure` | 11 | | `remote_ollama_reapply_and_drift_keep_bindings_and_stop_on_observation_failure` | 11 | | `managed_bearer_credentials_survive_recovery_reapply_and_replacement_outside_state` | 10 | | `managed_pi_applies_without_generation_and_refused_sandbox_changes_keep_intent` | 9 | | `managed_bearer_replacement_cleanup_recovers_and_teardown_accepts_both_identities` | 7 | | `remote_model_apply_recovers_failed_creation_and_lost_process_or_cache_keeping_data` | 7 | | `remote_ollama_apply_recovers_failed_creation_and_lost_process_or_cache_keeping_data` | 7 | | partial-destroy and pulled-image tests (4) | 0 | Several tests created and destroyed a runtime only to repeat checks that another scenario already ran against the same kind of runtime. ## Decision Run each check against one runtime per scenario, and only once. The Docker provider is unchanged. | Before | After | |---|---| | `remote_model_apply_recovers_failed_creation_and_lost_process_or_cache_keeping_data` | Removed. vLLM failed-creation, startup, lost-process, and cache recovery run in `managed_bearer_credentials_survive_recovery_reapply_and_replacement_outside_state`, against the same vLLM runtime with a bearer. Present-image compatibility runs in the Ollama and Pi tests, and the pull-policy plan runs in the Ollama test. | | `remote_ollama_apply_recovers_…` and `remote_ollama_reapply_and_drift_…` | `remote_ollama_recovers_reapplies_and_stops_on_observation_failure_keeping_data`: recovery, then export and reapply, the transport failure, and teardown against the recovered runtime. The readiness gate, compute replacement plan, and pull-policy reapply do not depend on the runtime kind and run against vLLM in `remote_model_reapply_and_drift_…`. `managed_installers_accept_current_runtime_readiness_without_collecting_model_files` in `nemoclaw-provider` covers each runtime's readiness observation. | | `managed_bearer_storage_or_key_damage_…` and `managed_bearer_replacement_cleanup_…` | `managed_bearer_cleanup_recovers_damage_keeps_bindings_and_teardown_accepts_both_identities`: the daemon retargeting, credential-volume damage, and insecure-key checks run against the state that replacement cleanup settles, before the teardown with both identities. | | Missing, foreign, and substituted credential volumes through CLI plan and apply | New `service_storage::missing_foreign_or_substituted_storage_stops_plan_and_apply_without_changing_state` contract test, for both storage kinds, through pinned OpenTofu against a fake engine. The lifecycle test keeps one substituted volume to show the CLI stops before changing bindings. | | Pi: failed-creation recovery, then an export after the last refusal | Pi applies once; the bearer and Ollama tests own recovery. The export after the last refusal is dropped: the intent is byte-identical to the one the earlier export already reproduced, and the destroy that follows reads the same state. | The partial-destroy and pulled-image tests are unchanged. ## Validation - Local `linux_arm64` run of `remote_service`, same request log: | | Tests | Network reads | Summed test time | Wall (4 threads) | |---|---|---|---|---| | Before | 12 | 75 | 404 s | 110 s | | After | 9 | 48 | 280 s | 97 s | After the change, by test: vLLM reapply 11, Ollama 9, bearer credentials 10, bearer cleanup and damage 12, Pi 6. - The new contract test asserts each refusal reason (`bound persistent storage is absent; recreation forbidden`, `observed ownership, generation, or durable identity changed`). - `remote_service` and `service_storage` lifecycle tests pass locally; `cargo fmt --all --check` and `cargo clippy --workspace --all-targets -- -D warnings` are clean. - `v1` at a49fc77 does not compile the provider contract tests (`kubernetes_lifecycle.rs`). This branch carries the same fix as #12981 in its own commit so CI can run, and merging `v1` after #12981 lands leaves one copy. - CI lifecycle JUnit reports from run [38100630144](https://github.com/NVIDIA/NemoClaw/actions/runs/38100630144), compared with the `v1` run for #12974 ([38098264065](https://github.com/NVIDIA/NemoClaw/actions/runs/38098264065)), the latest `v1` run whose lifecycle tests ran. Cells are partition wall seconds / summed test seconds / `remote_service` summed seconds: | Partition | Before | After | |---|---|---| | linux_arm64 / 1 | 198.8 / 788 / 175 | 200.2 / 795 / 171 | | linux_arm64 / 2 | 178.0 / 711 / 252 | 149.3 / 587 / 120 | | linux_amd64 / 1 | 221.0 / 878 / 209 | 208.0 / 825 / 186 | | linux_amd64 / 2 | 174.0 / 696 / 247 | 165.6 / 654 / 142 | `remote_service` went from 427 s to 291 s summed on linux_arm64, and from 456 s to 328 s on linux_amd64. Across the 9 `Rust desired-state` runs before #12974, it ranged 446–454 s on linux_arm64. The network read counts above come from the local request log; CI runs the same commands. The linux_arm64 lifecycle step is still bounded by partition 1, which kept its share of the `remote_service` tests and did not get shorter; partition 2 dropped 29 s.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #12882.
Failure
Nearly all lifecycle test time is spent inside SDK and CLI operations. I measured representative lifecycle tests against a locally built
linux_arm64bundle by logging every OpenTofu subprocess and everyProgress::Completedstep. Four tests (fabric_deployment::harness_pi,remote_service::managed_pi_applies_without_generation_and_refused_sandbox_changes_keep_intent,deployment::web_search_cli_export_reapply_and_destroy,deployment::rejected_policy_fails_promptly_with_context_and_allows_recovery_or_destroy) ran 30 SDK or CLI processes and 102.4 s of OpenTofu and bundle work:tofu plantofu plantofu applytofu plan -refresh-onlybundle.verifytofu show -json(bindings, saved plans, readiness)tofu initTwo items in that profile are repeated work:
tofu initandtofu show, then read them again before planning that stage.Decision
Both changes leave results and errors the same.
The largest remaining fixed cost is outside this change. The pinned
kreuzwerker/docker4.6.0 provider waits a fixed 2 s on everydocker_networkread and removal (networkReadRefreshDelayandnetworkRemoveRefreshDelay; upstreammasterstill has the same constants). Runtime plans, refreshes, creating applies, and destroys of a Docker-managed service each pay it. Inremote_service::remote_model_reapply_and_drift_keep_bindings_and_stop_on_observation_failure, 11 waits account for about 22 s of 62 s. Removing it needs a decision on how the network is managed:docker_networkstate to it without recreating the network.Validation
New
bundle::tests::every_listed_file_is_verified_and_the_first_listed_failure_is_reportedpins verification of every listed file and the error order. It passed before and after the change.deployment::destroy_does_not_require_the_inference_credential_or_rewrite_its_referencenow also asserts that plan-destroy and destroy each initialize the stage twice: once to read bindings and once for the teardown graph. It failed before the change (3 initializations) and passes after.Locally: the SDK
bundle::anddeployment::unit tests; lifecycle tests for teardown, Helm recovery, and theremote_service::managed_bearer_*scenarios;cargo fmt --all --check; andcargo clippy --workspace --all-targets -- -D warnings.The same four tests after the change: 96.3 s of OpenTofu and bundle work (was 102.4 s);
bundle.verify1.8 s (was 6.8 s); 49.4 s wall (was 55.7 s).CI lifecycle JUnit reports, run 38086127712, compared with the median of the 9 preceding successful
Rust desired-stateruns (wall seconds / summed test seconds):The linux_arm64 partitions varied by at most 3.7 s across those 9 runs, so their 8 s and 5 s drops are outside the noise. The linux_amd64 partitions varied by up to 31 s, so one run there does not show a change.